-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add new tip info for getting height #255
Conversation
WalkthroughThis update modifies dependency declarations in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant gRPCServer as ShaP2PoolGrpc
participant RXChain as RandomXChain
participant SHA3x as SHA3xChain
participant BaseNode
Client->>gRPCServer: get_tip_info(request)
gRPCServer->>RXChain: Retrieve tip info
RXChain-->>gRPCServer: [RandomX tip data]
gRPCServer->>SHA3x: Retrieve tip info
SHA3x-->>gRPCServer: [SHA3x tip data]
gRPCServer->>BaseNode: Retrieve tip info
BaseNode-->>gRPCServer: [Base node tip data]
gRPCServer->>Client: get_tip_info(response)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/server/grpc/p2pool.rs (2)
160-168
: Minor nitpick: Expand logging when appending uncles.
The code correctly assembles a list of uncles and appends them tonew_blocks
. Consider adding a debug or info log statement with the count of appended uncles for troubleshooting.let mut uncles = share_chain .get_blocks(&block.uncles) .await .into_iter() .map(Arc::<P2Block>::unwrap_or_clone) .collect(); + debug!(target: LOG_TARGET, "Appending {} uncles to the new block", uncles.len()); new_blocks.append(&mut uncles);
197-246
: Consider reusing the base node connection and handling partial errors more gracefully.
The newget_tip_info
method is generally well-structured and uses a timeout for reliability. However, reconnecting to the base node each time can incur overhead. Caching the gRPC client or gracefully handling partial fetch failures (e.g., still return share-chain tip data if base node fetch fails) could enhance robustness.Cargo.toml (1)
46-48
: Consider pinning dependency revisions for deterministic builds.
Switching from a fixed commit to a branch reference is suitable for ongoing development; however, it can introduce build unpredictability. If stability is a priority, pin each dependency with a commit SHA for reproducible builds.- minotari_app_grpc = { git = "https://github.com/stringhandler/tari.git", branch = "st-add-p2pool-tip-info" } + minotari_app_grpc = { git = "https://github.com/stringhandler/tari.git", rev = "f0f95a5" }Also applies to: 53-55, 57-58
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.toml
(1 hunks)src/server/grpc/p2pool.rs
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: machete
- GitHub Check: test (esmeralda)
- GitHub Check: clippy
- GitHub Check: cargo check with stable
🔇 Additional comments (2)
src/server/grpc/p2pool.rs (2)
19-19
: New imports for gRPC requests and responses look good.
They integrate well with theget_tip_info
method below without introducing breaking changes.Also applies to: 23-24
178-180
: Block rejection path is handled effectively.
Callingsend_miner_block_rejected
on the stats broadcast upon a missed tip is appropriate. No concerns here.
Summary by CodeRabbit
New Features
Chores
cargo-machete
tool in the CI workflow.